x86 hvm: Clean up VLAPIC interfaces a little, and fix vlapic_ipi().

A boolean flag was overflowing a uint8_t.

Thanks to Dongxiao Xu at Intel for tracking down the bug.

Signed-off-by: Keir Fraser <keir.fraser@citrix.com>

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index cc50cbca41ed93bb69333b97dfdd9f5dc1663f90..b915609d3662632c36e1006a867e2f4fa236561a 100644 (file)
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -296,35 +296,19 @@ static int vlapic_accept_sipi(struct vcpu *v, int trampoline_vector)
 }
 
 /* Add a pending IRQ into lapic. */
-static int vlapic_accept_irq(struct vcpu *v, int delivery_mode,
-                             int vector, int level, int trig_mode)
+static int vlapic_accept_irq(struct vcpu *v, uint32_t icr_low)
 {
     struct vlapic *vlapic = vcpu_vlapic(v);
+    uint8_t vector = (uint8_t)icr_low;
     int rc = X86EMUL_OKAY;
 
-    switch ( delivery_mode )
+    switch ( icr_low & APIC_MODE_MASK )
     {
     case APIC_DM_FIXED:
     case APIC_DM_LOWEST:
-        /* FIXME add logic for vcpu on reset */
-        if ( unlikely(!vlapic_enabled(vlapic)) )
-            break;
-
-        if ( vlapic_test_and_set_irr(vector, vlapic) && trig_mode )
-        {
-            HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
-                        "level trig mode repeatedly for vector %d", vector);
-            break;
-        }
-
-        if ( trig_mode )
-        {
-            HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
-                        "level trig mode for vector %d", vector);
-            vlapic_set_vector(vector, &vlapic->regs->data[APIC_TMR]);
-        }
-
-        vcpu_kick(v);
+        if ( vlapic_enabled(vlapic) &&
+             !vlapic_test_and_set_irr(vector, vlapic) )
+            vcpu_kick(v);
         break;
 
     case APIC_DM_REMRD:
@@ -342,7 +326,8 @@ static int vlapic_accept_irq(struct vcpu *v, int delivery_mode,
 
     case APIC_DM_INIT:
         /* No work on INIT de-assert for P4-type APIC. */
-        if ( trig_mode && !(level & APIC_INT_ASSERT) )
+        if ( (icr_low & (APIC_INT_LEVELTRIG | APIC_INT_ASSERT)) ==
+             APIC_INT_LEVELTRIG )
             break;
         rc = vlapic_accept_init(v);
         break;
@@ -352,8 +337,8 @@ static int vlapic_accept_irq(struct vcpu *v, int delivery_mode,
         break;
 
     default:
-        gdprintk(XENLOG_ERR, "TODO: unsupported delivery mode %x\n",
-                 delivery_mode);
+        gdprintk(XENLOG_ERR, "TODO: unsupported delivery mode in ICR %x\n",
+                 icr_low);
         domain_crash(v->domain);
     }
 
@@ -410,31 +395,21 @@ void vlapic_EOI_set(struct vlapic *vlapic)
 int vlapic_ipi(
     struct vlapic *vlapic, uint32_t icr_low, uint32_t icr_high)
 {
-    unsigned int dest =         GET_xAPIC_DEST_FIELD(icr_high);
-    unsigned int short_hand =   icr_low & APIC_SHORT_MASK;
-    unsigned int trig_mode =    icr_low & APIC_INT_LEVELTRIG;
-    unsigned int level =        icr_low & APIC_INT_ASSERT;
-    unsigned int dest_mode =    icr_low & APIC_DEST_MASK;
-    unsigned int delivery_mode =icr_low & APIC_MODE_MASK;
-    unsigned int vector =       icr_low & APIC_VECTOR_MASK;
-
+    unsigned int dest       = GET_xAPIC_DEST_FIELD(icr_high);
+    unsigned int short_hand = icr_low & APIC_SHORT_MASK;
+    unsigned int dest_mode  = !!(icr_low & APIC_DEST_MASK);
     struct vlapic *target;
     struct vcpu *v;
     int rc = X86EMUL_OKAY;
 
-    HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "icr_high 0x%x, icr_low 0x%x, "
-                "short_hand 0x%x, dest 0x%x, trig_mode 0x%x, level 0x%x, "
-                "dest_mode 0x%x, delivery_mode 0x%x, vector 0x%x",
-                icr_high, icr_low, short_hand, dest,
-                trig_mode, level, dest_mode, delivery_mode, vector);    
+    HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "icr = 0x%08x:%08x", icr_high, icr_low);
 
-    if ( delivery_mode == APIC_DM_LOWEST )
+    if ( (icr_low & APIC_MODE_MASK) == APIC_DM_LOWEST )
     {
         target = vlapic_lowest_prio(vlapic_domain(vlapic), vlapic,
                                     short_hand, dest, dest_mode);
         if ( target != NULL )
-            rc = vlapic_accept_irq(vlapic_vcpu(target), delivery_mode,
-                                   vector, level, trig_mode);
+            rc = vlapic_accept_irq(vlapic_vcpu(target), icr_low);
         return rc;
     }
 
@@ -442,9 +417,7 @@ int vlapic_ipi(
     {
         if ( vlapic_match_dest(vcpu_vlapic(v), vlapic,
                                short_hand, dest, dest_mode) )
-                rc = vlapic_accept_irq(v, delivery_mode,
-                                       vector, level, trig_mode);
-
+                rc = vlapic_accept_irq(v, icr_low);
         if ( rc != X86EMUL_OKAY )
             break;
     }

diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
index bbd2bfb3d4c21dc029360c34426e1d1556eabd96..24e38ebf6d85df5fc9a5a8ca6b6f49b2f987132f 100644 (file)
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -64,15 +64,12 @@ static void vmsi_inj_irq(
     }
 }
 
-#define VMSI_DEST_ID_MASK 0xff
 #define VMSI_RH_MASK      0x100
 #define VMSI_DM_MASK      0x200
 #define VMSI_DELIV_MASK   0x7000
 #define VMSI_TRIG_MODE    0x8000
 
-#define GFLAGS_SHIFT_DEST_ID        0
 #define GFLAGS_SHIFT_RH             8
-#define GFLAGS_SHIFT_DM             9
 #define GLFAGS_SHIFT_DELIV_MODE     12
 #define GLFAGS_SHIFT_TRG_MODE       15
 
@@ -81,8 +78,8 @@ int vmsi_deliver(struct domain *d, int pirq)
     struct hvm_irq_dpci *hvm_irq_dpci = d->arch.hvm_domain.irq.dpci;
     uint32_t flags = hvm_irq_dpci->mirq[pirq].gmsi.gflags;
     int vector = hvm_irq_dpci->mirq[pirq].gmsi.gvec;
-    uint16_t dest = (flags & VMSI_DEST_ID_MASK) >> GFLAGS_SHIFT_DEST_ID;
-    uint8_t dest_mode = (flags & VMSI_DM_MASK) >> GFLAGS_SHIFT_DM;
+    uint8_t dest = (uint8_t)flags;
+    uint8_t dest_mode = !!(flags & VMSI_DM_MASK);
     uint8_t delivery_mode = (flags & VMSI_DELIV_MASK) >> GLFAGS_SHIFT_DELIV_MODE;
     uint8_t trig_mode = (flags & VMSI_TRIG_MODE) >> GLFAGS_SHIFT_TRG_MODE;
     struct vlapic *target;